Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

errno: migrate test-infra to testify #26082

Merged
merged 8 commits into from
Jul 10, 2021
Merged

errno: migrate test-infra to testify #26082

merged 8 commits into from
Jul 10, 2021

Conversation

tisonkun
Copy link
Contributor

@tisonkun tisonkun commented Jul 9, 2021 •
edited
Loading

Signed-off-by: tison wander4096@gmail.com

What problem does this PR solve?

Issue Number:

What's Changed:

Migrate test-infra to testify for errno pkg.

Release note 

None

@tisonkun
errno: migrate test-infra to testify
95e8cf9 
Signed-off-by: tison <wander4096@gmail.com>
@ti-chi-bot ti-chi-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 9, 2021
@tisonkun
setup bridge between test infras
2fd97fe 
Signed-off-by: tison <wander4096@gmail.com>
tisonkun
tisonkun commented Jul 9, 2021
View reviewed changes
errno/infoschema_test.go Outdated Show resolved Hide resolved
@tisonkun
Copy link
Contributor Author

tisonkun commented Jul 9, 2021

/run-check_dev_2

tisonkun
tisonkun commented Jul 9, 2021
View reviewed changes
.build.ps1 Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@tisonkun
Copy link
Contributor Author

tisonkun commented Jul 9, 2021

/run-all-tests

@bb7133
Copy link
Member

bb7133 commented Jul 9, 2021

/run-unit-test

@tisonkun
improve bridges
9d49ce9 
Signed-off-by: tison <wander4096@gmail.com>
@ti-chi-bot ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 9, 2021
@tisonkun
license
e9fca63 
Signed-off-by: tison <wander4096@gmail.com>
xhebox
xhebox reviewed Jul 9, 2021
View reviewed changes
Copy link
Contributor

@xhebox xhebox left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will approve after response, whether or not accept the following suggestion:

Use testify suite here, and mark this PR as a reference PR in the tracking issue, though it is indeed not necessary to use a suite.

EDIT: fix check_dev, plz.

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 9, 2021
@tisonkun
Copy link
Contributor Author

tisonkun commented Jul 9, 2021

mark this PR as a reference PR in the tracking issue

@xhebox this pr already refers to subtasks of the tracking issue. We keep track on issues and do not lose the connection.

Also the connection is one a bidirectional tree, i.e.:

  1. tracking issue -> subtasks -> PRs
  2. PR -> subtask -> tracking issue

.. given the path 2 is present, it is redundant to refer PR -> tracking issue.

@xhebox
Copy link
Contributor

xhebox commented Jul 9, 2021 •
edited
Loading

this pr already refers to subtasks of the tracking issue.

Be clear, I mean an reference/example of testify suite API usage, instead of reference to tasks/issues.

@tisonkun
Copy link
Contributor Author

tisonkun commented Jul 9, 2021 •
edited
Loading

@xhebox testify suite runs serially inside, I'd prefer use as less suite as possible. You can follow the upstream thread stretchr/testify#187 for more information.

I'm going to wrap suite with go testing and use testify for its matchers when parallelism is required. Anyway, testify suite is not a net win and I'll trade off use it or not. We don't use it here for no benefit (although also no harm in this case).

@tisonkun
Copy link
Contributor Author

tisonkun commented Jul 9, 2021

this pr already refers to subtasks of the tracking issue.

Be clear, I mean an reference/example of testify suite API usage, instead of reference to tasks/issues.

Make sense. I will do it. Thanks for your suggestion!

@tisonkun tisonkun mentioned this pull request Jul 9, 2021
Closed
53 tasks
@tisonkun
Copy link
Contributor Author

tisonkun commented Jul 9, 2021

@xhebox comments resolved. PTAL when you have spare time :)

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • disksing
  • xhebox

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot removed the status/LGT1 Indicates that a PR has LGTM 1. label Jul 9, 2021
@xhebox
Copy link
Contributor

xhebox commented Jul 10, 2021

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 9d51c92

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 10, 2021
@tisonkun
Copy link
Contributor Author

/run-all-tests

1 similar comment
@tisonkun
Copy link
Contributor Author

/run-all-tests

@ti-chi-bot ti-chi-bot merged commit 98b2c88 into pingcap:master Jul 10, 2021
@tisonkun tisonkun mentioned this pull request Jul 10, 2021
Closed
@tisonkun tisonkun deleted the intro-testify branch July 10, 2021 07:51
@tisonkun tisonkun mentioned this pull request Jul 10, 2021
Closed
@tisonkun tisonkun mentioned this pull request Jul 18, 2021
Merged
@xhebox xhebox mentioned this pull request Jul 23, 2021
Merged
@ti-chi-bot ti-chi-bot mentioned this pull request Aug 25, 2021
Merged
This was referenced Aug 31, 2021
Merged
Closed
Merged
Merged
@qw4990
Copy link
Contributor

qw4990 commented Sep 17, 2021

/cherry-pick release-5.1

@ti-srebot ti-srebot mentioned this pull request Sep 17, 2021
Merged
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Sep 17, 2021
@tisonkun @ti-srebot
cherry pick pingcap#26082 to release-5.1
4e65ff9 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-5.1 in PR #28155

ti-chi-bot pushed a commit that referenced this pull request Sep 17, 2021
@ti-srebot
errno: migrate test-infra to testify (#26082) (#28155)
fb5a367
@tisonkun tisonkun mentioned this pull request Sep 17, 2021
Closed
21 tasks
@ti-chi-bot ti-chi-bot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Sep 24, 2021
@pingyu pingyu mentioned this pull request Oct 5, 2021
Merged
@zhaoxinyu zhaoxinyu mentioned this pull request Mar 22, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@disksing disksing disksing approved these changes

@xhebox xhebox xhebox approved these changes

@tiancaiamao tiancaiamao Awaiting requested review from tiancaiamao

Assignees

@xhebox xhebox

@disksing disksing

Labels
needs-cherry-pick-release-5.1 release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
7 participants
@tisonkun @bb7133 @xhebox @ti-chi-bot @qw4990 @ti-srebot @disksing